Skip to content

[FIX][SCHEMA] Phenotype subjects are a subset of participants.tsv - #2468

Merged
effigies merged 1 commit into
bids-standard:maint/1.11.1from
effigies:fix/reversed-subset-relation
Jul 20, 2026
Merged

[FIX][SCHEMA] Phenotype subjects are a subset of participants.tsv#2468
effigies merged 1 commit into
bids-standard:maint/1.11.1from
effigies:fix/reversed-subset-relation

Conversation

@effigies

@effigies effigies commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

The existing check required a superset relation. Probably not frequently hit because equality will be the norm.

Note that the error message implies a subset relation, and that was the intent in #2044.

A phenotype/ .tsv file lists subjects that were not found in the participant_id column found in the participants.tsv file.

Bug introduced in 1ff73de when moving away from an aggregated dataset.subjects.phenotype field.

@effigies
[FIX][SCHEMA] Phenotype subjects are a subset of participants.tsv
58c0373 
The existing check required a superset relation. Probably not frequently
hit because equality will be the norm.
@effigies
effigies requested review from ericearl and psadil July 17, 2026 15:26
@effigies
effigies requested a review from erdalkaraca as a code owner July 17, 2026 15:26
@effigies effigies added schema Issues related to the YAML schema representation of the specification. Patch version release. exclude-from-changelog This item will not feature in the automatically generated changelog labels Jul 17, 2026
psadil
psadil reviewed Jul 20, 2026
View reviewed changes

@psadil psadil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm regarding this change now expressing a subset. But, doesn't the message imply a set difference? I interpret the message as being equivalent to "The phenotype/ .tsv file contains subjects absent from participants.tsv (participant_id column)." Should it instead be something like "The phenotype/ .tsv file contains some (possibly all) participants listed in participants.tsv (participant_id column)."

@viranovskaya

viranovskaya commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

I think the existing diagnostic is consistent with the corrected relation. The validity condition is P_phenotype ⊆ P_participants, while the failure condition is P_phenotype \ P_participants ≠ ∅. So saying that the phenotype TSV contains subjects absent from participants.tsv describes the error correctly.
The proposed alternative (“contains some or all participants listed in participants.tsv”) would also describe a valid file.

“A phenotype TSV contains participant_id values that are absent from participants.tsv.”
It may also be worth testing both directions explicitly, since equality would hide the original reversal: {01,02} ⊆ {01,02,03} should pass, whereas phenotype {01,03} against participants {01,02} should fail.

@psadil

psadil commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

I think the existing diagnostic is consistent with the corrected relation. The validity condition is P_phenotype ⊆ P_participants, while the failure condition is P_phenotype \ P_participants ≠ ∅. So saying that the phenotype TSV contains subjects absent from participants.tsv describes the error correctly. The proposed alternative (“contains some or all participants listed in participants.tsv”) would also describe a valid file.

Ah, of course! Thanks for spelling that out.

@effigies

effigies commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for having a look, both!

@effigies
effigies merged commit 9e2ccac into bids-standard:maint/1.11.1 Jul 20, 2026
18 of 19 checks passed
@effigies
effigies deleted the fix/reversed-subset-relation branch July 20, 2026 16:24
yarikoptic added a commit to neuromechanist/bids-specification that referenced this pull request Aug 21, 2026
@yarikoptic
Merge branch 'master' into bep044-yahya-master
907d587 
* master: (27 commits)
  enh(metaschema): require and constrain `inherit` in associations + provide it for atlas _description.json (bids-standard#2479)
  chore: Bump schema package to 1.2.8-dev
  chore: Bump schema package to 1.2.7
  fix(ci): Drop test.pypi.org publication
  chore: Bump schema package to 1.2.7-dev
  chore: Bump schema package to 1.2.6
  fix(ci): Fix download-artifact target
  chore: Bump schema package to 1.2.6-dev
  chore: Bump schema package to 1.2.5
  [SCHEMA] Warn when *_beh.tsv contains onset and duration (bids-standard#2467)
  [FIX][SCHEMA] Phenotype subjects are a subset of participants.tsv (bids-standard#2468)
  [INFRA] Add Bluesky social link, drop Twitter (bids-standard#2470)
  [INFRA] Skip link check for encodeproject.org URLs (bids-standard#2469)
  [SCHEMA] Warn when *_beh.tsv contains onset and duration (bids-standard#2467)
  DOC: fix URL to Code of Conduct (bids-standard#2464)
  [ENH] Move PET DICOM correspondences into term definitions (bids-standard#2298)
  [ENH] Render phenotype sidecar table from schema (bids-standard#2324)
  [FIX] Replace non-existent `len` with existent `length` (bids-standard#2466)
  chore(deps): bump soupsieve from 2.8 to 2.8.4
  [pre-commit.ci] pre-commit autoupdate
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericearl ericearl Awaiting requested review from ericearl
@erdalkaraca erdalkaraca Awaiting requested review from erdalkaraca erdalkaraca is a code owner
1 more reviewer
@psadil psadil psadil left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

exclude-from-changelog This item will not feature in the automatically generated changelog schema Issues related to the YAML schema representation of the specification. Patch version release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@effigies @viranovskaya @psadil